Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Load script in body #10947

Merged
merged 2 commits into from
Mar 29, 2022
Merged

Load script in body #10947

merged 2 commits into from
Mar 29, 2022

Conversation

colin-grant-work
Copy link
Contributor

What it does

Fixes #10927 by loading the scripts in the body element rather than the head. This ensures that the body exists to be accessed when the JS runs.

How to test

  1. Remove --mode development from the bundle step in the examples/browser and examples/electron scripts.
  2. Build the application.
  3. Run the application in browser and Electron modes.
  4. It should load and work normally.

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work added the critical critical bugs / problems label Mar 29, 2022
vince-fugnitto
vince-fugnitto approved these changes Mar 29, 2022
View reviewed changes
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm that the changes work well following the updates 👍
As per offline discussions, an alternative would be to keep the script in the head but use defer.

msujew
msujew approved these changes Mar 29, 2022
View reviewed changes
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also works for me 👍

@colin-grant-work colin-grant-work merged commit 0bb62a2 into eclipse-theia:master Mar 29, 2022
@colin-grant-work colin-grant-work deleted the bugfix/production-mode branch March 29, 2022 17:26
@colin-grant-work colin-grant-work added this to the 1.24.0 milestone Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msujew msujew msujew approved these changes

@vince-fugnitto vince-fugnitto vince-fugnitto approved these changes

Assignees
No one assigned
Labels
critical critical bugs / problems
Projects
None yet
Milestone
1.24.0
Development

Successfully merging this pull request may close these issues.

Theia@Next: Production Build leads to null access in monaco-theming-service.ts
3 participants
@colin-grant-work @msujew @vince-fugnitto